build: Fix parallel build race on Analyzers.Common deps file#54
Conversation
The previous fix (#37) added GlobalPropertiesToRemove="TargetFramework" to all direct references to Analyzers.Common, but transient CI failures persisted. Investigation revealed two root causes: 1. Missing fix on Analyzers.UnitTests -> Analyzers reference: The 3-TFM test project (net10.0/net9.0/net8.0) passed its own TargetFramework to the multi-targeted Analyzers project without SetTargetFramework, creating 3 parallel build configurations that all converged on Analyzers.Common simultaneously. 2. Inconsistent ProjectReference strategies across consuming projects: FunctionalTests used SetTargetFramework="TargetFramework=netstandard2.0" on its Analyzers.Common reference while all other projects used GlobalPropertiesToRemove="TargetFramework". These produce different MSBuild global property sets ({TargetFramework=netstandard2.0} vs {}) for the same single-targeted project, creating two distinct build configurations both writing to the same output directory — the actual source of the race condition. The correct strategy depends on whether the referenced project is single-targeted or multi-targeted: - Single-targeted (Analyzers.Common): Use GlobalPropertiesToRemove to strip TargetFramework. This produces a config matching the solution's natural build, so MSBuild deduplicates correctly. - Multi-targeted (Analyzers, SourceGeneration): Use SetTargetFramework to pin to netstandard2.0. This matches one of the inner builds dispatched by the solution's outer multi-target build. Additionally, --graph is added to the CI build command as defense-in-depth. Graph builds evaluate the full project dependency graph upfront and ensure each project+config is built exactly once in topological order.
WalkthroughThree targeted configuration adjustments: CI workflow adds Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #54 +/- ##
=======================================
Coverage 87.66% 87.66%
=======================================
Files 35 35
Lines 2099 2099
Branches 344 344
=======================================
Hits 1840 1840
Misses 157 157
Partials 102 102 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/CompositeKey.Analyzers.UnitTests/CompositeKey.Analyzers.UnitTests.csproj (1)
38-38: Line 38 exceeds the 120-character limit — consider splitting the attributes.The changed
ProjectReferenceelement runs to ~146 characters. As per coding guidelines,**/*.{cs,csproj,props}files enforce a 120-character maximum line length.♻️ Proposed split
- <ProjectReference Include="..\CompositeKey.Analyzers\CompositeKey.Analyzers.csproj" SetTargetFramework="TargetFramework=netstandard2.0" /> + <ProjectReference + Include="..\CompositeKey.Analyzers\CompositeKey.Analyzers.csproj" + SetTargetFramework="TargetFramework=netstandard2.0" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/CompositeKey.Analyzers.UnitTests/CompositeKey.Analyzers.UnitTests.csproj` at line 38, The ProjectReference element on the given csproj exceeds 120 chars; split its attributes into multiple lines to keep each line under the limit — for example, break the single <ProjectReference Include="..\CompositeKey.Analyzers\CompositeKey.Analyzers.csproj" SetTargetFramework="TargetFramework=netstandard2.0" /> into a multi-line element by placing Include and SetTargetFramework on separate lines (or convert SetTargetFramework to a child element) so the ProjectReference tag and its attributes no longer exceed 120 characters.src/CompositeKey.SourceGeneration.FunctionalTests/CompositeKey.SourceGeneration.FunctionalTests.csproj (1)
36-36: Line 36 exceeds the 120-character limit — consider splitting the attributes.The updated
ProjectReferenceelement runs to ~207 characters. As per coding guidelines,**/*.{cs,csproj,props}files enforce a 120-character maximum line length.♻️ Proposed split
- <ProjectReference Include="..\CompositeKey.Analyzers.Common\CompositeKey.Analyzers.Common.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" GlobalPropertiesToRemove="TargetFramework" /> + <ProjectReference + Include="..\CompositeKey.Analyzers.Common\CompositeKey.Analyzers.Common.csproj" + OutputItemType="Analyzer" + ReferenceOutputAssembly="false" + GlobalPropertiesToRemove="TargetFramework" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/CompositeKey.SourceGeneration.FunctionalTests/CompositeKey.SourceGeneration.FunctionalTests.csproj` at line 36, The ProjectReference element on line 36 is over the 120-character limit; replace the single long attribute list with a multi-line ProjectReference element so each setting is on its own line. Change the one-line tag that contains Include="..\CompositeKey.Analyzers.Common\CompositeKey.Analyzers.Common.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" GlobalPropertiesToRemove="TargetFramework" into a block like <ProjectReference Include="..."> with child elements for OutputItemType, ReferenceOutputAssembly, and GlobalPropertiesToRemove so the attributes are split across multiple lines and no line exceeds 120 characters.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/CompositeKey.Analyzers.UnitTests/CompositeKey.Analyzers.UnitTests.csproj`:
- Line 38: The ProjectReference element on the given csproj exceeds 120 chars;
split its attributes into multiple lines to keep each line under the limit — for
example, break the single <ProjectReference
Include="..\CompositeKey.Analyzers\CompositeKey.Analyzers.csproj"
SetTargetFramework="TargetFramework=netstandard2.0" /> into a multi-line element
by placing Include and SetTargetFramework on separate lines (or convert
SetTargetFramework to a child element) so the ProjectReference tag and its
attributes no longer exceed 120 characters.
In
`@src/CompositeKey.SourceGeneration.FunctionalTests/CompositeKey.SourceGeneration.FunctionalTests.csproj`:
- Line 36: The ProjectReference element on line 36 is over the 120-character
limit; replace the single long attribute list with a multi-line ProjectReference
element so each setting is on its own line. Change the one-line tag that
contains
Include="..\CompositeKey.Analyzers.Common\CompositeKey.Analyzers.Common.csproj"
OutputItemType="Analyzer" ReferenceOutputAssembly="false"
GlobalPropertiesToRemove="TargetFramework" into a block like <ProjectReference
Include="..."> with child elements for OutputItemType, ReferenceOutputAssembly,
and GlobalPropertiesToRemove so the attributes are split across multiple lines
and no line exceeds 120 characters.
Summary
FunctionalTests→Analyzers.Common: changed fromSetTargetFrameworktoGlobalPropertiesToRemoveto match all other consuming projects. The mismatch created two distinct MSBuild build configurations ({TargetFramework=netstandard2.0}vs{}) for the same single-targeted project, both writing to the same output directory — the root cause of the race.SetTargetFrameworkonAnalyzers.UnitTests→Analyzersreference, preventing the 3-TFM test project from spawning 3 parallel builds of the multi-targeted Analyzers project.--graphto CI build as defense-in-depth, ensuring each project+config is built exactly once in topological order.Why the strategy differs by project type
Analyzers.Commonnetstandard2.0)GlobalPropertiesToRemove="TargetFramework"{}→ MSBuild deduplicatesAnalyzers,SourceGenerationnet8.0;netstandard2.0)SetTargetFramework="TargetFramework=netstandard2.0"Test plan
--graphbuilds pass consistentlyAnalyzers.Commonnow builds exactly once (was twice due to config mismatch)dotnet packproduces package successfullySummary by CodeRabbit
Release Notes